v2: Assign all nextProps dataset properties to chart#109
v2: Assign all nextProps dataset properties to chart#109austinpray merged 5 commits intoreactjs:chartjs-v2from ianks:label-fix-v2
Conversation
|
Anyone give this branch a whirl yet? |
|
I've installed this branch to test out v2 functionality. So far everything works great for me. Thanks for putting in that effort! |
|
It's working great for me, too! |
|
Works for me as well |
|
Tested with Doughnuts and Pie charts. Works great! |
| this.state.chart = Chart[chartType](ctx, { | ||
| data: nextProps.data, | ||
| options: nextProps.options | ||
| }); |
There was a problem hiding this comment.
Would it be better to use setState here? Thank you for this commit!
There was a problem hiding this comment.
@dougmolineux I would agree.
Even moving this.initializeChart from componentDidMount to componentWillMount to avoid unecessary re-render.
There was a problem hiding this comment.
I don't think this is possible since initializeChart relies on the chart being mounted.
There was a problem hiding this comment.
It also might not be possible to use setState since it is asynchronous, and the chart being available is required to be sync the way the code is currently written. I'd rather not turn this PR into a full re-factor.
|
@ianks can you make the changes mentioned in comments if you find them suitable? How can I assist? |
|
I'll make the changes when I get home tomorrow. |
Fixes chart creation on initializeChart
|
Does the new combining feature in v2 work with these changes? |
|
@russelldc I am not sure. I have not tested it. Can you give it a try? |
|
Hi @russelldc, I'm using combined charts with this branch and it works properly. |
|
It still does not work for me. Fix: in core.js replace with |
|
I get this error using chartjs:2.1.6 |
|
How do I install chart.js-v2 with npm to help with the testing ? |
|
@nesbtesh add this to your package.json |

This resolves my issue posted in #84
Essentially, only the data was being updated from the datasets. This PR makes its so all of the properties from the dataset are now assigned to the new chart. This will allow for things like:
It also re-introduces the
redrawprop to force a re-render of the canvas.I have only tested this with the Line chart, so if anyone has any production apps where there make use of Doughnut charts, etc. please give this branch a whirl!